Skip to content

Defer BLS signature decompression until verification - #427

Draft
sausagee wants to merge 1 commit into
m1from
cursor/defer-bls-decompression-1a4b
Draft

Defer BLS signature decompression until verification#427
sausagee wants to merge 1 commit into
m1from
cursor/defer-bls-decompression-1a4b

Conversation

@sausagee

@sausagee sausagee commented Sep 6, 2026

Copy link
Copy Markdown

Description

Clean-room rewrite on m1 (not a cherry-pick, patch, or mechanical port from aptos-labs/aptos-core). Upstream intent was read only to identify the bug and the invariant that must hold (aptos-labs/aptos-core#20089). types/src/lazy_bls.rs does not exist on this branch; this tree uses a different type (WireBlsSignature in types/src/wire_bls.rs) with a different structure, names, comments, and tests.

Bug: AggregateSignature and SignatureWithStatus deserialized through bls12381::Signature, which recovers a G2 point (field square-root) inside serde/TryFrom. A well-formed-length but invalid payload then failed to decode, so a later full-LedgerInfo commit-vote match could never inspect the ledger fields.

Invariant:

  • Network decode of these types accepts a 96-byte compressed payload (plus the historical Signature serde newtype). It does not recover a group element.
  • Commit-vote matching compares LedgerInfo / commit info only and must succeed without decompression.
  • Consensus safety is unchanged: bitmask and voting-power checks still run first; G2 recovery plus pairing verification still run before a vote or commit decision is accepted. Unrecoverable payloads fail verify/aggregate, never become a QC.
  • BCS and JSON remain bitwise identical to bls12381::Signature. Consensus-facing maps stay BTreeMap. No unsafe.

How Has This Been Tested?

  • cargo test -p aptos-types221 passed (includes new wire-compat tests, junk-payload decode, LedgerInfo match without decompression, plus existing validator_verifier / ledger_info / aggregator tests).
  • Focused filter (wire_bls, junk payload, signature status/aggregator, verifier): 21 passed.
  • cargo test -p aptos-consensus-types19 passed.
  • cargo test -p aptos-safety-rules13 passed.
  • cargo test -p aptos-consensus targeted: pending_votes (3), pending_order_votes (2), buffer_item (2), signing_phase (1) — all passed.

Aikido SAST was attempted on the new/modified files. The Aikido MCP is not authenticated in this environment (aikido_login requires a browser callback to localhost), so the scan could not complete here.

Key Areas to Review

  • types/src/wire_bls.rs — length-only decode; explicit recover_group_element.
  • AggregateSignature / SignatureWithStatus — store compressed bytes; verify paths recover after structural checks.
  • SignatureAggregator::try_aggregate — skips unrecoverable points; voting power is still checked on the stored voter set; verify_multi_signatures then checks the recovered bitmask.

Type of Change

  • Bug fix
  • Performance improvement
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Other (specify): aptos-types consensus signature types

Checklist

  • Clean-room implementation; no aptos-labs commit SHAs, patches, or copied function bodies
  • RUST_SECURE_CODING.md followed: no unsafe, BTreeMap for consensus maps, Result on recover
  • Named crate tests for changed crates
Open in Web Open in Cursor 

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Keep AggregateSignature and SignatureWithStatus in compressed 96-byte
wire form so LedgerInfo commit-vote matching is not blocked by G2
point recovery. Recover the group element only on verification and
aggregation paths. BCS/JSON encoding stays identical to bls12381::Signature.

This is a clean-room rewrite of the deferred-decompression invariant
for m1; it does not cherry-pick or port aptos-labs/aptos-core.

Co-authored-by: Young Yang Liauw <sausagee@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants